bors [Fri, 9 Mar 2018 19:40:03 +0000 (19:40 +0000)]
Auto merge of #5126 - gilescope:cycle-error-message, r=alexcrichton
Error message for package "depends on itself" lists the packages in the cycle.
I got a cycle while trying to build someone else's code and cargo's error message didn't point me in the right direction, just mentioned there was a cycle. I thought we could be a bit more helpful.
Don't know what you think of {:#?} as the display but it seemed minimal code so I thought I'd start with that. I could compress the output to one package per line if preferred.
bors [Fri, 9 Mar 2018 17:56:35 +0000 (17:56 +0000)]
Auto merge of #5157 - Eh2406:more_interning, r=alexcrichton
More interning
This is a forward approach to interning. Specifically `Dependency` and `PackageId` store their names as `InternedString`s and leave that value interned as long as possible. The alternative is to make a new `interned_name` function. The advantage of this approach is that a number of places in the code are doing `deb.name() == pid.name()` and are now using the fast pointer compare instead of the string compare, without the code needing to change. The disadvantage is that lots of places need to call `deref` with `&*` to convert to an `&str` and sum need to use `.to_inner()` to get a `&'static str`.
In a test on https://github.com/rust-lang/cargo/issues/4810#issuecomment-357553286
Before we got to 10000000 ticks in ~48 sec
After we got to 10000000 ticks in ~44 sec
bors [Thu, 8 Mar 2018 15:21:26 +0000 (15:21 +0000)]
Auto merge of #5150 - Eh2406:more_interning, r=alexcrichton
More interning
This is a small part of the unsuccessful last commit of #5121, this part removes `InternedString::new` from the innerest of loops.
This is mostly a resubmission of #5147, that I accidentally deleted while bors was testing. This one has new commits, so github will take the resubition.
bors [Thu, 8 Mar 2018 04:02:02 +0000 (04:02 +0000)]
Auto merge of #5147 - Eh2406:restucture, r=alexcrichton
restructure `Activations` for better clone
This builds on the work in #5121 When we last met we had: 5000000 ticks in ~48 sec, 0r 104k ticks/sec
This small change brings us to: 5000000 ticks in ~21 sec, 0r 238k ticks/sec
Edit: sorry for the large diff only the last commit is new. The rest are from #5121
bors [Wed, 7 Mar 2018 22:54:54 +0000 (22:54 +0000)]
Auto merge of #5121 - Eh2406:string_interning, r=alexcrichton
String interning
This builds on the work from #5118. This interns the strings in the part of resolver that gets cloned a lot.
In a test on https://github.com/rust-lang/cargo/issues/4810#issuecomment-357553286
Before we got to 1700000 ticks in ~(63 to 67) sec from #5118
After we got to 1700000 ticks in ~(42 to 45) sec
The interning code itself would be much better with a `leak` function that converts a `String` to a `&'static str`. Something like:
```rust
pub fn leek(s: String) -> &'static str {
let ptr = s.as_ptr();
let len = s.len();
mem::forget(s);
unsafe {
let slice = slice::from_raw_parts(ptr, len);
str::from_utf8(slice).unwrap()
}
}
```
but "there is no unsafe in Cargo", and I am not the best at unsafe. So I just `to_string` and lived with the extra copy. Is there a better way to hand out references?
I assumed that `InternedString::new` world start appearing in profile result, and that we would want `PackageId`, and `Summary`, Et Al. to store the `InternedString`. That is why I put the interner in a shared folder. So far it is just used in the resolver. It may make sense for a lot more of the Strings to be interned, but with the extra copy... I have not explored it yet.
bors [Tue, 6 Mar 2018 22:34:32 +0000 (22:34 +0000)]
Auto merge of #5135 - matklad:drop-ignored-tests, r=alexcrichton
Drop ignored tests
r? @alexcrichton
These tests are ignored, so its better to remove them. `run` does not supports `--bins` argument, so I've left a single test that checks specifically for this.
Alex Crichton [Tue, 6 Mar 2018 21:36:05 +0000 (13:36 -0800)]
Optimize SourceId::crates_io
Turns out this gets called a lot in large projects as almost all dependencies
come from crates.io and parsed manifests use this. Let's cache the result as
it's always the same!
Eh2406 [Sun, 4 Mar 2018 03:38:10 +0000 (22:38 -0500)]
intern the features
In a test on https://github.com/rust-lang/cargo/issues/4810#issuecomment-357553286
Before we got to 5000000 ticks in ~65 sec
After we got to 5000000 ticks in ~52 sec
Eh2406 [Sun, 4 Mar 2018 02:01:33 +0000 (21:01 -0500)]
make a global string interner
In a test on https://github.com/rust-lang/cargo/issues/4810#issuecomment-357553286
Before we got to 5000000 ticks in ~72 sec
After we got to 5000000 ticks in ~65 sec
bors [Tue, 6 Mar 2018 19:16:55 +0000 (19:16 +0000)]
Auto merge of #5132 - Eh2406:less_clones, r=alexcrichton
don't clone a BacktrackFrame if we are not going to backtrack
I think this is a fix for #5130.
Currently the only recoverable `activate` error does not corrupt `cx`, so this is definitely ok. We should be careful as we add more recoverable `activate` errors that they don't corrupt `cx` to the point where the error messages are affected.
bors [Tue, 6 Mar 2018 03:01:47 +0000 (03:01 +0000)]
Auto merge of #5122 - acmcarther:acm-emit-features-from-cargo-metadata, r=alexcrichton
Emit Resolve.features_sorted in "cargo metadata"
This PR adds `features` to `metadata.resolve.nodes[*]` using the Resolve object's known features. This is different from the `features` fields that already exist elsewhere within metadata as this one is the actual features that need to be used in order to build the code.
Context: I'm currently using Cargo's internals to synthesize BUILD files for the Bazel build tool. `cargo metadata` currently provides almost everything I need in order to avoid using Cargo's internals -- *except* for this.
bors [Sun, 4 Mar 2018 20:47:56 +0000 (20:47 +0000)]
Auto merge of #5118 - Eh2406:more_rc, r=alexcrichton
use more Rc in the part of resolver that gets cloned a lot
In a test on https://github.com/rust-lang/cargo/issues/4810#issuecomment-357553286
Before we got to 1700000 ticks in ~(82 to 97) sec
After we got to 1700000 ticks in ~(63 to 67) sec
Eh2406 [Sun, 4 Mar 2018 02:33:23 +0000 (21:33 -0500)]
use more Rc in the part of resolver that gets cloned a lot
In a test on https://github.com/rust-lang/cargo/issues/4810#issuecomment-357553286
Before we got to 1700000 ticks in ~(82 to 97) sec
After we got to 1700000 ticks in ~(63 to 67) sec
bors [Sat, 3 Mar 2018 21:32:24 +0000 (21:32 +0000)]
Auto merge of #5112 - Eh2406:cache_queries, r=alexcrichton
Cache the query result.
Small performance gain.
In a test on https://github.com/rust-lang/cargo/issues/4810#issuecomment-357553286
Before we got to 1700000 ticks in ~97 sec
After we got to 1700000 ticks in ~92 sec. I just reran and got ~82, so it seems to be unstable.
And query disappears from the flame graph
bors [Fri, 2 Mar 2018 23:37:29 +0000 (23:37 +0000)]
Auto merge of #5110 - alexcrichton:reset-harder, r=matklad
Try a lot harder to recover corrupt git repos
We've received a lot of intermittent bug reports historically about corrupt git
repositories. These inevitably happens as Cargo is ctrl-c'd or for whatever
other reason, and to provide a better user experience Cargo strives to
automatically handle these situations by blowing away the old checkout for a new
update.
This commit adds a new test which attempts to pathologically corrupt a git
database and checkout in an attempt to expose bugs in Cargo. Sure enough there
were some more locations that we needed to handle gracefully for corrupt git
checkouts. Notable inclusions were:
* The `fetch` operation in libgit2 would fail due to corrupt references. This
starts by adding an explicit whitelist for classes of errors coming out of
`fetch` to auto-retry by blowing away the repository. We need to be super
careful here as network errors commonly come out of this function and we don't
want to too aggressively re-clone.
* After a `fetch` succeeded a repository could fail to actual resolve a
git reference to the actual revision we want. This indicated that we indeed
needed to blow everything away and re-clone entirely again.
* When creating a checkout from a database the `reset` operation might fail due
to a corrupt local database of the checkout itself. If this happens we needed
to just blow it away and try again.
There's likely more lurking situations where we need to re-clone but I figure we
can discover those over time.
Alex Crichton [Fri, 2 Mar 2018 17:44:47 +0000 (09:44 -0800)]
Try a lot harder to recover corrupt git repos
We've received a lot of intermittent bug reports historically about corrupt git
repositories. These inevitably happens as Cargo is ctrl-c'd or for whatever
other reason, and to provide a better user experience Cargo strives to
automatically handle these situations by blowing away the old checkout for a new
update.
This commit adds a new test which attempts to pathologically corrupt a git
database and checkout in an attempt to expose bugs in Cargo. Sure enough there
were some more locations that we needed to handle gracefully for corrupt git
checkouts. Notable inclusions were:
* The `fetch` operation in libgit2 would fail due to corrupt references. This
starts by adding an explicit whitelist for classes of errors coming out of
`fetch` to auto-retry by blowing away the repository. We need to be super
careful here as network errors commonly come out of this function and we don't
want to too aggressively re-clone.
* After a `fetch` succeeded a repository could fail to actual resolve a
git reference to the actual revision we want. This indicated that we indeed
needed to blow everything away and re-clone entirely again.
* When creating a checkout from a database the `reset` operation might fail due
to a corrupt local database of the checkout itself. If this happens we needed
to just blow it away and try again.
There's likely more lurking situations where we need to re-clone but I figure we
can discover those over time.
Eh2406 [Fri, 2 Mar 2018 22:56:36 +0000 (17:56 -0500)]
Cache the query result.
In a test on https://github.com/rust-lang/cargo/issues/4810#issuecomment-357553286
Before we got to 1700000 ticks in ~97 sec
After we got to 1700000 ticks in ~92 sec
And query disappears from the flame graph
bors [Fri, 2 Mar 2018 18:43:11 +0000 (18:43 +0000)]
Auto merge of #5095 - tdbgamer:master, r=matklad
Issue #5087
* remove Workspace::current_manifest
* remove incorrect logging
* move empty check to Packages::into_package_id_specs
* add test case mentioned in issue
bors [Thu, 1 Mar 2018 21:49:10 +0000 (21:49 +0000)]
Auto merge of #5100 - alexcrichton:better-dep-ordering, r=matklad
Improve Cargo's scheduling of builds
Historically Cargo has been pretty naive about scheduling builds, basically just
greedily scheduling as much work as possible. As pointed out in #5014, however,
this isn't guaranteed to always have the best results. If we've got a very deep
dependency tree that would otherwise fill up our CPUs Cargo should ideally
schedule these dependencies first. That way when we reach higher up in the
dependency tree we should have more work available to fill in the cracks if
there's spare cpus.
Alex Crichton [Thu, 1 Mar 2018 06:04:54 +0000 (22:04 -0800)]
Improve Cargo's scheduling of builds
Historically Cargo has been pretty naive about scheduling builds, basically just
greedily scheduling as much work as possible. As pointed out in #5014, however,
this isn't guaranteed to always have the best results. If we've got a very deep
dependency tree that would otherwise fill up our CPUs Cargo should ideally
schedule these dependencies first. That way when we reach higher up in the
dependency tree we should have more work available to fill in the cracks if
there's spare cpus.
bors [Thu, 1 Mar 2018 21:27:17 +0000 (21:27 +0000)]
Auto merge of #5000 - Eh2406:i4347, r=alexcrichton
backtrack if can not activate
This is a fix for #4347
Unfortunately this too regressed error messages for the case that you specified a dependency feature that does not exist.
@alexcrichton advice on improving the message?
bors [Thu, 1 Mar 2018 19:48:18 +0000 (19:48 +0000)]
Auto merge of #5103 - alexcrichton:no-hamcrest, r=matklad
Drop outdated hamcrest dependency
This hasn't been updated in awhile and in general we've been barely using it.
This drops the outdated dependency and vendors a small amount of the
functionality that it provided. I think eventually we'll want to transition away
from this method of assertions but I wanted to get this piece in to avoid too
much churn in one commit.
Alex Crichton [Thu, 1 Mar 2018 18:14:17 +0000 (10:14 -0800)]
Drop outdated hamcrest dependency
This hasn't been updated in awhile and in general we've been barely using it.
This drops the outdated dependency and vendors a small amount of the
functionality that it provided. I think eventually we'll want to transition away
from this method of assertions but I wanted to get this piece in to avoid too
much churn in one commit.
Eh2406 [Tue, 27 Feb 2018 21:42:39 +0000 (16:42 -0500)]
fix the todo's
needs a test where we have an activation_error the then try activate something that dose not work and backtrack to where we had the activation_error then:
- Hit fast backtracking that go past the crate with the missing features
- Or give a bad error message that does not mention the activation_error.
The test will pass, but there is code that is not yet justified by tests
bors [Wed, 28 Feb 2018 15:20:30 +0000 (15:20 +0000)]
Auto merge of #5097 - matklad:end-of-tls-saga-hopefully, r=alexcrichton
Revert "Warn Windows 7 users about old TLS"
We now have upgraded libgit2 version, so Cargo would use TLS 1.2
on windows unconditionally. Note that even *unpatched* windows 7
supports TLS 1.2.
@retep998, you've originally written that
>TLS 1.2 support for Windows 7 still requires a rather recent patch and so even if we do fix that, we should still keep this error message because some people use Windows 7 with automatic updates disabled.
However, it looks like Windows 7 **does** support TLS 1.2 out of the box, and the update is required to enable the use of TLS 1.2 by default:
Alex Crichton [Tue, 27 Feb 2018 15:50:05 +0000 (07:50 -0800)]
Respect lock files in crates.io crates
Currently Cargo doesn't publish lock files in crates.io crates but we'll
eventually be doing so, so this changes Cargo to recognize `Cargo.lock` when
it's published to crates.io as use it as the basis for resolution during `cargo
install`.
bors [Tue, 27 Feb 2018 04:24:19 +0000 (04:24 +0000)]
Auto merge of #5059 - tobywhughes:master, r=alexcrichton
Clean with --verbose now prints path of what is being removed.
Added verbose output for the clean command. Simply takes whatever is passed into the rm_rf() function and outputs "Removing /whatever/the/path/to/remove/is"
[Issue this is being used for](https://github.com/rust-lang/cargo/issues/5056)
It makes `cargo --version` to print the corresponding Rust version (with possibly cargo-specific patch number), while keeping the library version the same as today.
On the one hand, this is horrible. On the other hand, it seems to do the job? In the long term, it would be cool skip one version bump for the library, so that `0.x` corresponds to `1.x`.
cc @rust-lang/cargo
I am not sure that this is good idea, the implementation certainly feels horrible :)
bors [Mon, 26 Feb 2018 19:59:27 +0000 (19:59 +0000)]
Auto merge of #5069 - matklad:win7-y-u-no-tls, r=alexcrichton
Warn Windows 7 users about old TLS
An eyepatch for https://github.com/rust-lang/cargo/issues/5066.
@retep998 what would the best way to check for windows 7 specifically? Currently I use just `#[cfg(windows)]`, which might give false positives on 8 and 10.